SOLR-17600 Replaces MapSerializable with MapWriter #4011

isaric · 2026-01-06T09:19:37Z

https://issues.apache.org/jira/browse/SOLR-17600

Description

This pull request replaces the deprecated MapSerializable interface with MapWriter across the Solr codebase. MapSerializable relied on the toMap(Map) pattern, which often led to unnecessary object allocations and inconsistent serialization behavior. By migrating to MapWriter and its writeMap(EntryWriter) pattern, we improve memory efficiency and ensure more consistent serialization across different formats like JavaBin and JSON.

Solution

The migration involved several key steps:

Interface Removal: Deleted the MapSerializable interface and removed its usage from the MapWriter interface. MapWriter now serves as the primary interface for object-to-map serialization.
Refactoring Components: Updated numerous classes across the codebase to implement MapWriter instead of MapSerializable. Key areas include:
- Core Configuration: SolrConfig, PluginInfo, SolrIndexConfig, CacheConfig, ConfigOverlay, RequestParams, IndexSchema.
- Cloud & Admin APIs: Many V2 API response classes such as CreateCore, InstallShardData, MigrateDocsAPI, ModifyCollectionAPI, MoveReplicaAPI, SplitShardAPI, and more.
- Internal State: ZkNodeProps, IndexFingerprint.
Method Migration: Converted toMap(Map<String, Object> map) implementations to writeMap(EntryWriter ew). This allows for direct writing to the underlying data structure (like JavaBinCodec's output stream) without necessarily creating intermediate Map objects.
Utility Updates: Updated Utils.convertToMap, Utils.reflectToMap, and other utility methods to support the new pattern. Utils.makeDeepCopy now uses SimpleOrderedMap with MapWriter for better efficiency.
Clean up: Removed redundant toMap calls and simplified serialization logic in several handlers and components. Updated JavaBinCodec, TextWriter, and JSONWriter to optimize for MapWriter.

Tests

Existing tests were updated to accommodate the change in serialization:

Updated TestConfig, SolrIndexConfigTest, CacheConfigTest, and NodeConfigClusterPluginsSourceTest to verify that the configuration objects are still correctly serialized.
Updated TestSchemaDesignerAPI and SolrExampleCborTest to ensure API and serialization consistency.
Verified that JavaBinCodec and JSONWriter correctly handle the new MapWriter implementations.
Ran core Solr tests to ensure no regressions in response formatting or internal state representation.

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide
I have added a changelog entry for my change

… serialization

…across Solr codebase

…` across Solr codebase

…serialization consistency

…proved map serialization consistency

…pWriter

…d serialization consistency

…Map` for consistency in plugin and handler serialization

…plugin serialization consistency

…tter readability and consistency

…deredMap` for consistent serialization in config and handler classes

…plugin and component info serialization with `SimpleOrderedMap`

epugh · 2026-01-06T12:13:56Z

solr/core/src/java/org/apache/solr/core/RequestParams.java

    Map deepCopy = Utils.getDeepCopy(data, 3);
    Map p = (Map) deepCopy.get(NAME);
    if (p == null) deepCopy.put(NAME, p = new LinkedHashMap<>());
    if (paramSet == null) p.remove(name);


I wonder if this if should have { } to match the else?

I agree that in Java conditionals should always use curly braces for clarity. This is the existing code that was not changed in my PR.

I am hesitant to change it as I am not sure if there is a convention regarding this practice in the project.

epugh · 2026-01-06T12:17:27Z

solr/core/src/java/org/apache/solr/core/SolrConfig.java

-          "cacheControl",
-          cacheControlHeader);
+    public void writeMap(EntryWriter ew) throws IOException {
+      ew.put("never304", never304)


this is nicer pattern, less a "unique magic pattern".

dsmiley · 2026-01-07T06:25:39Z

solr/core/src/java/org/apache/solr/handler/admin/api/CreateCore.java


  public static CreateCoreParams createRequestBodyFromV1Params(SolrParams solrParams) {
-    final var v1ParamMap = solrParams.toMap(new HashMap<>());
+    final var v1ParamMap = new SimpleOrderedMap<>(solrParams);


There is a subtle semantic difference here. Previously, we had a bonafide Map. If there were duplicate keys, the last value wins (looking at Utils.convertToMap, called by toMap). Probably wasn't intended but there is integrity of the Map contract at least. The constructor you use assumes the input has no duplicate keys. They will be added, and SimpleOrderedMap will not quite act exactly like a Map. I'm not sure what the final effect is without trying/testing. We could change SimpleOrderedMap's constructor here to have a hack for the NamedList (that isn't SimpleOrderedMap) case. Or don't use SimpleOrderedMap here, which I can see was chosen for your convenience, not because it's needed.

Replaced with Utils.convertToMap.

…CreateCore` for parameter handling simplification

dsmiley

Aside from just removing a redundant abstraction, this change should have the benefit of removing intermediate needless data structures (Maps/NamedList/SimpleOrderedMap). But I'm seeing here a continuation of that.

The only place we need the data structure is where toMap was being called. Perhaps it'd be ideal to split this huge PR in two -- first address toMap callers, then the rest.

dsmiley · 2026-01-09T05:22:24Z

solr/core/src/java/org/apache/solr/api/NodeConfigClusterPluginsSource.java

-          Map<String, Object> config = p.initArgs.toMap(new HashMap<>());
+          Map<String, Object> config = new SimpleOrderedMap<>(p.initArgs);


Again, avoid NamedList/SimpleOrderedMap

How about p.initArgs.asMap(0)

Changed as requested

dsmiley · 2026-01-09T05:33:03Z

solr/core/src/java/org/apache/solr/cloud/api/collections/InstallShardDataCmd.java

        CoreAdminParams.ACTION, CoreAdminParams.CoreAdminAction.INSTALLCOREDATA.toString());
-    typedMessage.toMap(new HashMap<>()).forEach((k, v) -> coreApiParams.set(k, v.toString()));
-
+    new SimpleOrderedMap<>(typedMessage).forEach((k, v) -> coreApiParams.set(k, v.toString()));


typedMessage._forEachEntry here instead
(avoiding senseless intermediate data structure creation).

dsmiley · 2026-01-09T05:37:34Z

solr/core/src/java/org/apache/solr/core/PluginInfo.java

+    new NamedList<>(attributes).writeMap(ew);
+    if (initArgs != null) {
+      new SimpleOrderedMap<>(initArgs).writeMap(ew);
+    }


Let's not create a new Map just to ultimately write it!

Changed caused test failures. Unsure why. Error was cannot cast LinkedHashMap to String however stack trace was unclear as to location of exception.
Reverted change.

dsmiley · 2026-01-09T05:43:10Z

solr/core/src/java/org/apache/solr/core/RequestParams.java

    if (paramSet == null) p.remove(name);
-    else p.put(name, paramSet.toMap(new LinkedHashMap<>()));
+    else {
+      p.put(name, new SimpleOrderedMap<>(paramSet));


This one's fine

…ap serialization and entry iteration to simplify code and improve readability

…:core

isaric · 2026-01-09T17:11:19Z

@dsmiley I followed the advice in the comments however I had test failures in solr:core after pushing changes. Reverted changes to PluginInfo and tests are passing on my Codespaces instance. I don't think the solr:core tests ran for the last commit. I guess it is restricted due to heavy resource use.

dsmiley · 2026-01-10T04:19:18Z

changelog/unreleased/SOLR-17600.yml

@@ -0,0 +1,7 @@
+title: SOLR 17600 - Replace MapSerializable with MapWriter
+type: changed


refactorings are "other".
I'm not happy with the logchange plugin constraining the enum choices here

dsmiley · 2026-01-10T04:34:33Z

solr/core/src/java/org/apache/solr/core/SolrConfig.java

+        ew.put(
+            tag,
+            new SimpleOrderedMap<>(
+                m -> {
+                  new NamedList<>(items).writeMap(m);
+                }));


not sure what I'm looking at here

dsmiley · 2026-01-10T04:42:57Z

solr/core/src/java/org/apache/solr/handler/admin/api/InstallShardData.java


    messageTyped.validate();
-    return new ZkNodeProps(messageTyped.toMap(new HashMap<>()));
+    return new ZkNodeProps((Map<String, Object>) new SimpleOrderedMap<>(messageTyped));


generally, we should avoid creating a SimpleOrderedMap. Use Utils.convertToMap instead.
A SimpleOrderedMap is okay to add to a MapWritable entry though, or any similar place for a response-writing use-case. A SimpleOrderedMap does not hash its data, so has O(N) access. It's designed for writing out data that will unlikely have its keys looked up. It's not a general replacement for a Map.

isaric added 13 commits January 6, 2026 10:12

SOLR-17600: Replace MapSerializable with MapWriter across Solr codebase

629b684

SOLR-17600: Add toMap method override in IndexSchema to support map…

3645172

… serialization

SOLR-17600: Migrate remaining toMap usages to MapWriter.writeMap …

02c3ef6

…across Solr codebase

SOLR-17600: Replace MapWriter.writeMap usage with `SimpleOrderedMap…

a37c1d5

…` across Solr codebase

SOLR-17600: Replace toMap calls with SimpleOrderedMap to improve …

8b53f86

…serialization consistency

SOLR-17600: Refactor to use NamedList and SimpleOrderedMap for im…

5224a46

…proved map serialization consistency

SOLR-17600: Add changelog entry for replacing MapSerializable with Ma…

e671468

…pWriter

SOLR-17600: Refactor writeMap to use SimpleOrderedMap for improve…

32da08a

…d serialization consistency

SOLR-17600: Refactor writeMap implementations to use `SimpleOrdered…

234d79b

…Map` for consistency in plugin and handler serialization

SOLR-17600: Refactor to use lists of SimpleOrderedMap for improved …

1ac31a6

…plugin serialization consistency

SOLR-17600: Improve formatting and indentation in SolrConfig for be…

d3d763d

…tter readability and consistency

SOLR-17600: Replace MapWriter and PluginInfo usage with `SimpleOr…

409136c

…deredMap` for consistent serialization in config and handler classes

SOLR-17600: Refactor SolrConfigHandler and SolrConfig to improve …

0e46609

…plugin and component info serialization with `SimpleOrderedMap`

github-actions bot added test-framework client:solrj module:extraction tests cat:search cat:cloud cat:index cat:api cat:schema cat:packagemanager labels Jan 6, 2026

epugh reviewed Jan 6, 2026

View reviewed changes

dsmiley reviewed Jan 7, 2026

View reviewed changes

SOLR-17600: Replace SimpleOrderedMap with Utils.convertToMap in `…

4fb8c2f

…CreateCore` for parameter handling simplification

dsmiley reviewed Jan 9, 2026

View reviewed changes

isaric and others added 2 commits January 9, 2026 15:57

SOLR-17600: Replace SimpleOrderedMap with more direct methods for m…

cee7b37

…ap serialization and entry iteration to simplify code and improve readability

SOLR-17600: Revert changes in PluginInfo due to test failures in solr…

1ea968a

…:core

dsmiley reviewed Jan 10, 2026

View reviewed changes

		Map<String, Object> config = p.initArgs.toMap(new HashMap<>());
		Map<String, Object> config = new SimpleOrderedMap<>(p.initArgs);

		@@ -0,0 +1,7 @@
		title: SOLR 17600 - Replace MapSerializable with MapWriter
		type: changed

SOLR-17600 Replaces MapSerializable with MapWriter #4011

Are you sure you want to change the base?

SOLR-17600 Replaces MapSerializable with MapWriter #4011

Conversation

isaric commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Tests

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isaric commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

isaric commented Jan 6, 2026 •

edited

Loading